[Diagnostics] Return rvalue reference from temporary argument #127400

hahnjo · 2025-02-16T15:09:02Z

This fixes compilation issues with GCC and C++23:

error: cannot bind non-const lvalue reference of type
'llvm::OptimizationRemarkMissed&' to an rvalue of type
'llvm::OptimizationRemarkMissed'

Closes #105778

llvmbot · 2025-02-16T15:09:33Z

@llvm/pr-subscribers-llvm-ir

Author: Jonas Hahnfeld (hahnjo)

Changes

This fixes compilation issues with GCC and C++23:

error: cannot bind non-const lvalue reference of type
'llvm::OptimizationRemarkMissed&amp;' to an rvalue of type
'llvm::OptimizationRemarkMissed'

Closes #105778

Full diff: https://github.com/llvm/llvm-project/pull/127400.diff

3 Files Affected:

(modified) llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h (+5)
(modified) llvm/include/llvm/IR/DiagnosticInfo.h (+18-6)
(modified) llvm/lib/Analysis/InlineAdvisor.cpp (+7-1)

diff --git a/llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h b/llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h
index 7b14e55782adb..2640bb9194010 100644
--- a/llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h
+++ b/llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h
@@ -69,6 +69,11 @@ class OptimizationRemarkEmitter {
   /// Output the remark via the diagnostic handler and to the
   /// optimization record file.
   void emit(DiagnosticInfoOptimizationBase &OptDiag);
+  /// Also allow r-value for OptDiag to allow emitting a temporarily-constructed
+  /// diagnostic.
+  void emit(DiagnosticInfoOptimizationBase &&OptDiag) {
+    emit(static_cast<DiagnosticInfoOptimizationBase &>(OptDiag));
+  }
 
   /// Take a lambda that returns a remark which will be emitted.  Second
   /// argument is only used to restrict this to functions.
diff --git a/llvm/include/llvm/IR/DiagnosticInfo.h b/llvm/include/llvm/IR/DiagnosticInfo.h
index 694785317af04..1e26cfdd17b5f 100644
--- a/llvm/include/llvm/IR/DiagnosticInfo.h
+++ b/llvm/include/llvm/IR/DiagnosticInfo.h
@@ -30,6 +30,7 @@
 #include <iterator>
 #include <optional>
 #include <string>
+#include <utility>
 
 namespace llvm {
 
@@ -625,14 +626,14 @@ operator<<(RemarkT &R,
 /// Also allow r-value for the remark to allow insertion into a
 /// temporarily-constructed remark.
 template <class RemarkT>
-RemarkT &
+RemarkT &&
 operator<<(RemarkT &&R,
            std::enable_if_t<
                std::is_base_of<DiagnosticInfoOptimizationBase, RemarkT>::value,
                StringRef>
                S) {
   R.insert(S);
-  return R;
+  return std::move(R);
 }
 
 template <class RemarkT>
@@ -647,14 +648,14 @@ operator<<(RemarkT &R,
 }
 
 template <class RemarkT>
-RemarkT &
+RemarkT &&
 operator<<(RemarkT &&R,
            std::enable_if_t<
                std::is_base_of<DiagnosticInfoOptimizationBase, RemarkT>::value,
                DiagnosticInfoOptimizationBase::Argument>
                A) {
   R.insert(A);
-  return R;
+  return std::move(R);
 }
 
 template <class RemarkT>
@@ -669,14 +670,14 @@ operator<<(RemarkT &R,
 }
 
 template <class RemarkT>
-RemarkT &
+RemarkT &&
 operator<<(RemarkT &&R,
            std::enable_if_t<
                std::is_base_of<DiagnosticInfoOptimizationBase, RemarkT>::value,
                DiagnosticInfoOptimizationBase::setIsVerbose>
                V) {
   R.insert(V);
-  return R;
+  return std::move(R);
 }
 
 template <class RemarkT>
@@ -690,6 +691,17 @@ operator<<(RemarkT &R,
   return R;
 }
 
+template <class RemarkT>
+RemarkT &&
+operator<<(RemarkT &&R,
+           std::enable_if_t<
+               std::is_base_of<DiagnosticInfoOptimizationBase, RemarkT>::value,
+               DiagnosticInfoOptimizationBase::setExtraArgs>
+               EA) {
+  R.insert(EA);
+  return std::move(R);
+}
+
 /// Common features for diagnostics dealing with optimization remarks
 /// that are used by IR passes.
 class DiagnosticInfoIROptimization : public DiagnosticInfoOptimizationBase {
diff --git a/llvm/lib/Analysis/InlineAdvisor.cpp b/llvm/lib/Analysis/InlineAdvisor.cpp
index 12553dd446a61..2041610cd5ee9 100644
--- a/llvm/lib/Analysis/InlineAdvisor.cpp
+++ b/llvm/lib/Analysis/InlineAdvisor.cpp
@@ -338,7 +338,7 @@ static raw_ostream &operator<<(raw_ostream &R, const ore::NV &Arg) {
 }
 
 template <class RemarkT>
-RemarkT &operator<<(RemarkT &&R, const InlineCost &IC) {
+RemarkT &operator<<(RemarkT &R, const InlineCost &IC) {
   using namespace ore;
   if (IC.isAlways()) {
     R << "(cost=always)";
@@ -352,6 +352,12 @@ RemarkT &operator<<(RemarkT &&R, const InlineCost &IC) {
     R << ": " << ore::NV("Reason", Reason);
   return R;
 }
+
+template <class RemarkT>
+RemarkT &&operator<<(RemarkT &&R, const InlineCost &IC) {
+  static_cast<RemarkT &>(R) << IC;
+  return std::move(R);
+}
 } // namespace llvm
 
 std::string llvm::inlineCostStr(const InlineCost &IC) {

llvmbot · 2025-02-16T15:09:34Z

@llvm/pr-subscribers-llvm-analysis

Author: Jonas Hahnfeld (hahnjo)

Changes

This fixes compilation issues with GCC and C++23:

error: cannot bind non-const lvalue reference of type
'llvm::OptimizationRemarkMissed&amp;' to an rvalue of type
'llvm::OptimizationRemarkMissed'

Closes #105778

Full diff: https://github.com/llvm/llvm-project/pull/127400.diff

3 Files Affected:

(modified) llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h (+5)
(modified) llvm/include/llvm/IR/DiagnosticInfo.h (+18-6)
(modified) llvm/lib/Analysis/InlineAdvisor.cpp (+7-1)

diff --git a/llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h b/llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h
index 7b14e55782adb..2640bb9194010 100644
--- a/llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h
+++ b/llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h
@@ -69,6 +69,11 @@ class OptimizationRemarkEmitter {
   /// Output the remark via the diagnostic handler and to the
   /// optimization record file.
   void emit(DiagnosticInfoOptimizationBase &OptDiag);
+  /// Also allow r-value for OptDiag to allow emitting a temporarily-constructed
+  /// diagnostic.
+  void emit(DiagnosticInfoOptimizationBase &&OptDiag) {
+    emit(static_cast<DiagnosticInfoOptimizationBase &>(OptDiag));
+  }
 
   /// Take a lambda that returns a remark which will be emitted.  Second
   /// argument is only used to restrict this to functions.
diff --git a/llvm/include/llvm/IR/DiagnosticInfo.h b/llvm/include/llvm/IR/DiagnosticInfo.h
index 694785317af04..1e26cfdd17b5f 100644
--- a/llvm/include/llvm/IR/DiagnosticInfo.h
+++ b/llvm/include/llvm/IR/DiagnosticInfo.h
@@ -30,6 +30,7 @@
 #include <iterator>
 #include <optional>
 #include <string>
+#include <utility>
 
 namespace llvm {
 
@@ -625,14 +626,14 @@ operator<<(RemarkT &R,
 /// Also allow r-value for the remark to allow insertion into a
 /// temporarily-constructed remark.
 template <class RemarkT>
-RemarkT &
+RemarkT &&
 operator<<(RemarkT &&R,
            std::enable_if_t<
                std::is_base_of<DiagnosticInfoOptimizationBase, RemarkT>::value,
                StringRef>
                S) {
   R.insert(S);
-  return R;
+  return std::move(R);
 }
 
 template <class RemarkT>
@@ -647,14 +648,14 @@ operator<<(RemarkT &R,
 }
 
 template <class RemarkT>
-RemarkT &
+RemarkT &&
 operator<<(RemarkT &&R,
            std::enable_if_t<
                std::is_base_of<DiagnosticInfoOptimizationBase, RemarkT>::value,
                DiagnosticInfoOptimizationBase::Argument>
                A) {
   R.insert(A);
-  return R;
+  return std::move(R);
 }
 
 template <class RemarkT>
@@ -669,14 +670,14 @@ operator<<(RemarkT &R,
 }
 
 template <class RemarkT>
-RemarkT &
+RemarkT &&
 operator<<(RemarkT &&R,
            std::enable_if_t<
                std::is_base_of<DiagnosticInfoOptimizationBase, RemarkT>::value,
                DiagnosticInfoOptimizationBase::setIsVerbose>
                V) {
   R.insert(V);
-  return R;
+  return std::move(R);
 }
 
 template <class RemarkT>
@@ -690,6 +691,17 @@ operator<<(RemarkT &R,
   return R;
 }
 
+template <class RemarkT>
+RemarkT &&
+operator<<(RemarkT &&R,
+           std::enable_if_t<
+               std::is_base_of<DiagnosticInfoOptimizationBase, RemarkT>::value,
+               DiagnosticInfoOptimizationBase::setExtraArgs>
+               EA) {
+  R.insert(EA);
+  return std::move(R);
+}
+
 /// Common features for diagnostics dealing with optimization remarks
 /// that are used by IR passes.
 class DiagnosticInfoIROptimization : public DiagnosticInfoOptimizationBase {
diff --git a/llvm/lib/Analysis/InlineAdvisor.cpp b/llvm/lib/Analysis/InlineAdvisor.cpp
index 12553dd446a61..2041610cd5ee9 100644
--- a/llvm/lib/Analysis/InlineAdvisor.cpp
+++ b/llvm/lib/Analysis/InlineAdvisor.cpp
@@ -338,7 +338,7 @@ static raw_ostream &operator<<(raw_ostream &R, const ore::NV &Arg) {
 }
 
 template <class RemarkT>
-RemarkT &operator<<(RemarkT &&R, const InlineCost &IC) {
+RemarkT &operator<<(RemarkT &R, const InlineCost &IC) {
   using namespace ore;
   if (IC.isAlways()) {
     R << "(cost=always)";
@@ -352,6 +352,12 @@ RemarkT &operator<<(RemarkT &&R, const InlineCost &IC) {
     R << ": " << ore::NV("Reason", Reason);
   return R;
 }
+
+template <class RemarkT>
+RemarkT &&operator<<(RemarkT &&R, const InlineCost &IC) {
+  static_cast<RemarkT &>(R) << IC;
+  return std::move(R);
+}
 } // namespace llvm
 
 std::string llvm::inlineCostStr(const InlineCost &IC) {

github-actions · 2025-02-16T15:12:57Z

✅ With the latest revision this PR passed the C/C++ code formatter.

This fixes compilation issues with GCC and C++23: ``` error: cannot bind non-const lvalue reference of type 'llvm::OptimizationRemarkMissed&' to an rvalue of type 'llvm::OptimizationRemarkMissed' ``` Closes llvm#105778

dwblaikie

Looks like OK changes.

I wouldn't mind a unit test for something low level/broadly used like this, but wouldn't insist on it.

llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h

hahnjo · 2025-03-11T21:06:32Z

I wouldn't mind a unit test for something low level/broadly used like this, but wouldn't insist on it.

I generally agree, but unfortunately there is none for OptimizationRemarkEmitter and I'm not really familiar with the code (just trying to address compilation errors when experimenting with C++23) 😧

llvm/include/llvm/IR/DiagnosticInfo.h

kuhar

LGTM % nit

llvm/include/llvm/IR/DiagnosticInfo.h

hahnjo · 2025-03-13T07:24:24Z

@arsenm thanks for merging. However, for future changes I would prefer to do this myself, both to decide on the author email address (work or private; this merge got it right because I contributed privately outside of work hours) and to make sure I'm available to respond to any issues reported by the bots...

hahnjo requested review from arsenm, qcolombet and anemet February 16, 2025 15:09

llvmbot added llvm:ir llvm:analysis labels Feb 16, 2025

Return rvalue reference from temporary argument

3b0573b

This fixes compilation issues with GCC and C++23: ``` error: cannot bind non-const lvalue reference of type 'llvm::OptimizationRemarkMissed&' to an rvalue of type 'llvm::OptimizationRemarkMissed' ``` Closes llvm#105778

hahnjo force-pushed the diagnostic-rvalue branch from e0e37c9 to 3b0573b Compare February 16, 2025 15:17

hahnjo mentioned this pull request Feb 17, 2025

C++23 crash in DiagnosticInfo.h root-project/root#17686

Open

1 task

hahnjo requested a review from vitalybuka February 25, 2025 09:59

vitalybuka removed their request for review February 27, 2025 05:30

hahnjo requested a review from nikic March 10, 2025 16:50

nikic requested review from kuhar and dwblaikie March 10, 2025 16:57

nikic changed the title ~~Return rvalue reference from temporary argument~~ [Diagnostics] Return rvalue reference from temporary argument Mar 10, 2025

dwblaikie approved these changes Mar 10, 2025

View reviewed changes

llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h Outdated Show resolved Hide resolved

Remove unneeded cast

824936c

kuhar reviewed Mar 11, 2025

View reviewed changes

llvm/include/llvm/IR/DiagnosticInfo.h Outdated Show resolved Hide resolved

kuhar requested a review from zero9178 March 11, 2025 21:36

Unify duplicated template functions

0a80551

hahnjo requested a review from kuhar March 12, 2025 18:38

kuhar approved these changes Mar 12, 2025

View reviewed changes

Use is_base_of_v

60c7cae

arsenm merged commit 44ff94e into llvm:main Mar 13, 2025
9 of 10 checks passed

hahnjo deleted the diagnostic-rvalue branch March 13, 2025 07:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Diagnostics] Return rvalue reference from temporary argument #127400

[Diagnostics] Return rvalue reference from temporary argument #127400

hahnjo commented Feb 16, 2025

llvmbot commented Feb 16, 2025

llvmbot commented Feb 16, 2025

github-actions bot commented Feb 16, 2025 •

edited

Loading

dwblaikie left a comment

hahnjo commented Mar 11, 2025

kuhar left a comment

hahnjo commented Mar 13, 2025

[Diagnostics] Return rvalue reference from temporary argument #127400

[Diagnostics] Return rvalue reference from temporary argument #127400

Conversation

hahnjo commented Feb 16, 2025

llvmbot commented Feb 16, 2025

llvmbot commented Feb 16, 2025

github-actions bot commented Feb 16, 2025 • edited Loading

dwblaikie left a comment

Choose a reason for hiding this comment

hahnjo commented Mar 11, 2025

kuhar left a comment

Choose a reason for hiding this comment

hahnjo commented Mar 13, 2025

github-actions bot commented Feb 16, 2025 •

edited

Loading